Skip to content

Conversation

mgol
Copy link
Member

@mgol mgol commented Aug 11, 2025

Changes:

  • Checkboxradio: Change .attr( "checked", true ) to `.attr( "checked", "checked" )
  • Selectmenu: Disable the boolean-attributes patch for one assertion where it's impossible to avoid

@mgol mgol added this to the 1.14.2 milestone Aug 11, 2025
@mgol mgol requested review from fnagel and timmywil August 11, 2025 22:09
@mgol mgol self-assigned this Aug 11, 2025
@mgol mgol force-pushed the migrate-4-tests branch from eb3167f to ac886c1 Compare August 11, 2025 22:14
fnagel
fnagel previously requested changes Aug 12, 2025
Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 by reading, but a small typo needs to be fixed

Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending @fnagel's comment

mgol added 2 commits August 12, 2025 22:01
Changes:
* Checkboxradio: Change `.attr( "checked", true )` to
  `.attr( "checked", "checked" )
* Selectmenu: Disable the `boolean-attributes` patch for one assertion where
  it's impossible to avoid
@mgol mgol force-pushed the migrate-4-tests branch from ac886c1 to ac6bc89 Compare August 12, 2025 20:01
@mgol mgol requested a review from fnagel August 12, 2025 20:02
@mgol
Copy link
Member Author

mgol commented Aug 12, 2025

@fnagel PTAL

@mgol
Copy link
Member Author

mgol commented Aug 13, 2025

@fnagel BTW, if you generally approve a PR but just want a simple typo to be fixed in an obvious way then I'd prefer if you approve with a comment. I am not merging PRs blindly, I read comments first, so I'd fix the text, but then I'd be able to merge immediately, without waiting for another review.

@mgol mgol dismissed fnagel’s stale review August 18, 2025 20:12

I'll dismiss your review, @fnagel, since from your comment it seemed you only had this single remark which I fixed, Timmy approved the PR and almost a week have passed.

@mgol mgol merged commit a7ac081 into jquery:main Aug 18, 2025
11 checks passed
@mgol mgol deleted the migrate-4-tests branch August 18, 2025 20:12
@mgol mgol removed the Needs review label Aug 18, 2025
@fnagel
Copy link
Member

fnagel commented Aug 19, 2025

@fnagel BTW, if you generally approve a PR but just want a simple typo to be fixed in an obvious way then I'd prefer if you approve with a comment. I am not merging PRs blindly, I read comments first, so I'd fix the text, but then I'd be able to merge immediately, without waiting for another review.

Sure, I will do next time.

nikunj8866 pushed a commit to nikunj8866/jquery-ui that referenced this pull request Sep 9, 2025
Changes:
* Checkboxradio: Change `.attr( "checked", true )` to
  `.attr( "checked", "checked" )
* Selectmenu: Disable the `boolean-attributes` patch for one assertion where
  it's impossible to avoid

Closes jquerygh-2364
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants